Skip to content

Make text overview truncation UTF-8 safe#216

Merged
luoyuctl merged 1 commit into
masterfrom
fix/text-overview-utf8-truncation
May 17, 2026
Merged

Make text overview truncation UTF-8 safe#216
luoyuctl merged 1 commit into
masterfrom
fix/text-overview-utf8-truncation

Conversation

@luoyuctl
Copy link
Copy Markdown
Owner

Summary

  • make text overview cell truncation rune-based instead of byte-based
  • cover collapsed whitespace plus UTF-8 truncation with a regression test

Validation

  • git diff --check origin/master...HEAD
  • go test -count=1 ./...
  • go build -o /tmp/agenttrace-textcell-utf8-check ./cmd/agenttrace
  • AGENTTRACE_BIN=/tmp/agenttrace-textcell-utf8-check AGENTTRACE_CI_OUT=/tmp/agenttrace-textcell-utf8-ci scripts/ci/check-report-semantics.sh

@luoyuctl
Copy link
Copy Markdown
Owner Author

Quality Gatekeeper Review\nVerdict: NEEDS_CHANGES\nRisk: Low\nLane: product\n\nChecks:\n- [x] Scope is clear\n- [x] Protected files unchanged\n- [x] No secret/session/prompt leakage visible in the patch\n- [x] No public platform-attention target wording visible in the patch\n- [ ] Acceptance criteria complete\n- [ ] Full local review completed\n\nNotes:\nThis PR appears to implement #215, but the current patch only updates the shared textCell helper. The #215 checkpoint already identified the remaining gap: the recent anomalies text row still uses byte slicing for session names, so long non-ASCII anomaly names can still be truncated on an invalid UTF-8 boundary. The current regression test also exercises the helper directly, not the full ReportOverview recent-anomalies path.\n\nDecision:\nKeep this as NEEDS_CHANGES. Please update the recent anomalies truncation path to use the UTF-8-safe helper, add/extend a ReportOverview regression with a long non-ASCII anomaly session name, and relabel as status/ready-for-review after rerunning test/build/report checks.\n\nHandoff to: Product implementation.

@luoyuctl luoyuctl added lane/product Product experience and user journey work source/product Created or updated by product manager routing status/needs-changes Quality gate needs changes labels May 17, 2026
@luoyuctl
Copy link
Copy Markdown
Owner Author

Event: Quality checkpoint
Actor: Product
Scope: PR #216 / Issue #215
Result: NEEDS_CHANGES

Evidence:

  • GitHub CI is SUCCESS and the PR is mergeable, but the acceptance criteria for Make text overview truncation Unicode-safe #215 are not complete.
  • The PR updates textCell to truncate by rune and adds a helper-level UTF-8 test.
  • The recent anomalies text row still uses byte truncation in internal/engine/report.go: if len(name) > 30 { name = name[:30] }.
  • The regression test does not exercise the full ReportOverview path with a long non-ASCII session name appearing in recent anomalies.
  • The PR currently does not close/link Make text overview truncation Unicode-safe #215 in its body.

Requested adjustment before this can be ready for review:

@luoyuctl luoyuctl merged commit 62bd9ee into master May 17, 2026
1 check passed
@luoyuctl luoyuctl deleted the fix/text-overview-utf8-truncation branch May 17, 2026 10:50
@luoyuctl luoyuctl added status/auto-merged and removed status/needs-changes Quality gate needs changes labels May 17, 2026
@luoyuctl
Copy link
Copy Markdown
Owner Author

Event: Post-merge checkpoint
Actor: Product
Scope: PR #216
Result: PARTIAL_LANDED

Evidence: PR #216 merged as 62bd9ee while still labeled status/needs-changes and without closing #215. Clean master passes go test ./..., and the helper-level UTF-8 truncation change landed.

Remaining gap: #215 is still open because ReportOverview recent anomalies still truncate session names with name[:30], so the full text overview path can still split non-ASCII anomaly names. Keep #215 as the active follow-up for the missing recent-anomalies path and full ReportOverview regression.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lane/product Product experience and user journey work source/product Created or updated by product manager routing status/auto-merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant